Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Leverage micromatch to speed up glob matching #911

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Leverage micromatch to speed up glob matching #911

wants to merge 5 commits into from

Conversation

mastrzyz
Copy link

micromatch is faster than minimatch in performing matching.

Which can be seen with auditable benchmarks here

I wanted to be 100% sure this would help this project and created two benchmark files of my own here trying to replicate some of the usecases of this project.

The results are :

marcinstrzyz@Marcins-MacBook-Pro-2 lib % node benchmarkLarge.js
Fastest is micromatch
┌─────────┬──────────────┬────────────────┬───────────────────────┬────────────────────────────┬──────────────────────────┐
│ (index) │   Function   │ Mean Time (ms) │ Operations per Second │ Standard Error of the Mean │ Relative Margin of Error │
├─────────┼──────────────┼────────────────┼───────────────────────┼────────────────────────────┼──────────────────────────┤
│    0    │ 'micromatch' │    '23.75'     │        '42.10'        │           '0.00'           │          '4.11'          │
│    1    │ 'minimatch'  │    '71.48'     │        '13.99'        │           '0.00'           │          '9.11'          │
└─────────┴──────────────┴────────────────┴───────────────────────┴────────────────────────────┴──────────────────────────┘
marcinstrzyz@Marcins-MacBook-Pro-2 lib % node benchmark.js     
Fastest is micromatch
┌─────────┬──────────────┬────────────────┬───────────────────────┬────────────────────────────┬──────────────────────────┐
│ (index) │   Function   │ Mean Time (ms) │ Operations per Second │ Standard Error of the Mean │ Relative Margin of Error │
├─────────┼──────────────┼────────────────┼───────────────────────┼────────────────────────────┼──────────────────────────┤
│    0    │ 'micromatch' │     '0.03'     │      '37700.76'       │           '0.00'           │          '3.83'          │
│    1    │ 'minimatch'  │     '0.06'     │      '15809.91'       │           '0.00'           │          '6.07'          │
└─────────┴──────────────┴────────────────┴───────────────────────┴────────────────────────────┴──────────────────────────┘

Although the perceived speedup may be only seen in very large cases, we do see it is 50% improvement overall.

The current UT's work fine, happy to extend them if we have some worries about known edge cases or we just want more security.

@mastrzyz
Copy link
Author

@kenotron thoughts on this? wondering if Lage could benefit it further downstream as well.

@mastrzyz
Copy link
Author

@ecraig12345 ?

@ecraig12345
Copy link
Member

My big concern with making this change in a non-major version is the difference in backslash handling. A lot of our internal teams use Windows, so it wouldn't be surprising if somebody is using backslashes as path separators in patterns, and it's hard to accurately detect and fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mastrzyz @ecraig12345